Skip to content

Conversation

@birdcar
Copy link

@birdcar birdcar commented Jan 6, 2026

Description

Previously, when returning data from a resource that was paginated, we would parse the pagination args off the response, loop over the "data" value in the response, and map each item in the JSON array to a specific resource. An example of this would be in the listUsers() method of the UserManagement class (truncated example below):

$users = [];

list($before, $after) = Util\Request::parsePaginationArgs($response);
foreach ($response["data"] as $responseData) {
    \array_push($users, Resource\User::constructFromResponse($responseData));
}

return [$before, $after, $users];

Performing this pattern over and over again resulted in a lot of duplicate code that was doing basically nothing more than an array_map.

Additionally, this return is extremely limited and forces the user into a limited and specific pattern of either bare array destructuring:

[$before, $after, $users] = $userManagement->listUsers();

Or dealing with 0-indexed array values:

$result = $userManagement->listUsers();
$before = $result[0];
$after = $result[1];
$users = $result[2];

If for example they just want the first 5 users and don't care about paginating, this means they'd need to either write a destructuring expression that has empty values:

// Huh?
[,,$users] = $userManagement->listUsers(limit: 5);

Or they'd have to drop down to:

$results = $userManagement->listUsers(limit: 5);
// How do I discover or know what this index is?
$users = $results[2];

To fix both of these issues, without affecting current library consumers, I'm proposing that we create a Resource\PaginatedResource class that:

  1. DRYs and standardizes the creation of a paginated resource (e.g. now you can just return Resource\PaginatedResource::constructFromResponse($response, Resource\User::class, 'users');)
  2. Handles the resource mapping from the data array
  3. Continues to allow for bare destructuring (backwards compatible)
  4. Introduces named destructuring (e.g $result["after"] or ["users" => $fiveUsers] = $userManagement->listUsers(limit:5))
  5. Introduces fluent property access (e.g. $result->after or $result->users)

The change is fully backwards compatible, cleans up existing resource code and allows for developers to use the library in whichever code style is consistent with their project.

For example, it lets you turn this code:

[$before, $after, $users] = $userManagement->listUsers();

while ($after) {
    [$before, $after, $currentPage] = $sso->listConnections(
        limit: 100,
        after: $after,
        order: "desc"
    );

    $users = array_merge($users, $currentPage);
}

Into this code:

$users = [];
$after = null;

do {
    $result = $userManagement->listUsers(after: $after, limit: 100, order: "desc");
    $users = array_merge($users, $result->users);
    $after = $result->after;
} while ($after !== null);

Documentation

Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.

[ ] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

@birdcar birdcar self-assigned this Jan 6, 2026
@birdcar birdcar requested a review from a team as a code owner January 6, 2026 23:50
@birdcar birdcar requested a review from mattgd January 6, 2026 23:50
greptile-apps[bot]

This comment was marked as outdated.

@workos workos deleted a comment from greptile-apps bot Jan 6, 2026
Previously, when returning data from a resource that was paginated, we
would parse the pagination args off the resposne, loop over the "data"
value in the response, and map each item in the JSON array to a
specific resource. An example of this would be in the listUsers() method
of the UserManagement class (truncated example below):

```php
$users = [];

list($before, $after) = Util\Request::parsePaginationArgs($response);
foreach ($response["data"] as $responseData) {
    \array_push($users, Resource\User::constructFromResponse($responseData));
}

return [$before, $after, $users];
```

Performing this pattern over and over again resulted in a lot of
duplicate code that was doing basically nothing more than an array_map.

Additionally, this return is extremely limited and forces the user into
a limited and specific pattern of either bare array destructuring:

```php
[$before, $after, $users] = $userManagement->listUsers();
```

Or dealing with 0-indexed array values:

```php
$result = $userManagement->listUsers();
```

If for example they just want the first 5 users and don't care about paginating,
this means they'd to either write destructuring that has empty values:

```php
// Huh?
[,,$users] = $userManagement->listUsers(limit: 5);
```

Or they'd have to drop down to

```php
$results = $userManagement->listUsers(limit: 5);
// How do I discover or know what this index is?
$users = $results[2];
```

To fix both of these issues, without affecting current library
consumers, I'm proposing that we create a `Resource\PaginatedResource` class that:

1. DRYs and standardizes the creation of a paginated resource
2. Handles the resource mapping from the data array
3. Continues to allow for bare destructuring (backwards compatible)
4. Introduces named destructuring (e.g `$result["after"]` or
`["users" => $fiveUsers] = $userManagement->listUsers(limit:5)`)
5. Introduces fluent property access (e.g. `$result->after` or
`$result->users`)

The change is fully backwards compatible, cleans up existing resource
code and allows for developers to use the library in whichever code
style is consistent with their project.

For example, it lets you turn this code:

```php
[$before, $after, $users] = $userManagement->listUsers();

while ($after) {
    [$before, $after, $currentPage] = $sso->listConnections(
        limit: 100,
        after: $after,
        order: "desc"
    );

    $users = array_merge($users, $currentPage);
}
```

Into this code:

```php
$users = [];
$after = null;

do {
    $result = $userManagement->listUsers(after: $after, limit: 10);
    $users = array_merge($allUsers, $result->users);
    $after = $result->after;
} while ($after !== null);
```
@birdcar birdcar force-pushed the birdcar/better-paginated-resources branch from 30b94ee to d35cb34 Compare January 7, 2026 00:12
@birdcar
Copy link
Author

birdcar commented Jan 7, 2026

@greptileai

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 7, 2026

Greptile Summary

This PR introduces PaginatedResource, a new utility class that standardizes pagination handling across all WorkOS PHP SDK list methods and provides three flexible access patterns for library consumers.

Key Changes:

  • New PaginatedResource class - Implements ArrayAccess and IteratorAggregate to support backwards-compatible bare destructuring [$before, $after, $data], named destructuring ["users" => $users], and fluent property access $result->users
  • Refactored 9 list methods - Replaced duplicate pagination code in listDirectories(), listGroups(), listUsers(), listOrganizations(), listOrganizationFeatureFlags(), listConnections(), listOrganizationMemberships(), listInvitations(), and listVaultObjects() to use the new PaginatedResource::constructFromResponse() helper
  • Comprehensive test coverage - Added tests verifying all three access patterns work correctly for each refactored method
  • Code style improvements - UserManagement.php includes unrelated formatting changes (double to single quotes, spacing adjustments)

Benefits:

  • Eliminates code duplication across pagination methods
  • Provides developer-friendly API with multiple access patterns
  • Fully backwards compatible with existing code
  • Improves maintainability with centralized pagination logic

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is well-designed, fully backwards compatible, and thoroughly tested. The PaginatedResource class properly implements ArrayAccess and IteratorAggregate interfaces with correct behavior for all access patterns. All resource classes inherit from BaseWorkOSResource which provides the required constructFromResponse() static method. Comprehensive test coverage validates all three access patterns across all refactored methods. The refactoring reduces code duplication and improves maintainability without introducing breaking changes.
  • No files require special attention

Important Files Changed

Filename Overview
lib/Resource/PaginatedResource.php New class that provides backwards-compatible paginated resource handling with multiple access patterns (bare destructuring, named access, fluent properties)
lib/DirectorySync.php Refactored listDirectories, listGroups, and listUsers methods to use PaginatedResource, removing duplicate pagination code
lib/Organizations.php Refactored listOrganizations and listOrganizationFeatureFlags to use PaginatedResource
lib/SSO.php Refactored listConnections to use PaginatedResource
lib/UserManagement.php Refactored listUsers, listOrganizationMemberships, and listInvitations to use PaginatedResource; also includes code style improvements (string quotes, spacing)
lib/Vault.php Refactored listVaultObjects to use PaginatedResource

Sequence Diagram

sequenceDiagram
    participant Client as Client Code
    participant API as API Class<br/>(UserManagement/SSO/etc)
    participant HTTPClient as HTTP Client
    participant PaginatedResource as PaginatedResource
    participant ResourceClass as Resource Class<br/>(User/Directory/etc)

    Client->>API: listUsers(params)
    API->>HTTPClient: request(GET, path, params)
    HTTPClient-->>API: response JSON
    API->>PaginatedResource: constructFromResponse(response, Resource\User::class, 'users')
    
    PaginatedResource->>PaginatedResource: parsePaginationArgs(response)
    Note over PaginatedResource: Extracts before/after cursors<br/>from list_metadata
    
    loop For each item in response["data"]
        PaginatedResource->>ResourceClass: User::constructFromResponse(item)
        ResourceClass-->>PaginatedResource: User instance
    end
    
    PaginatedResource->>PaginatedResource: new PaginatedResource(before, after, data, 'users')
    PaginatedResource-->>API: PaginatedResource instance
    API-->>Client: PaginatedResource
    
    Note over Client: Multiple access patterns available:
    Client->>PaginatedResource: Bare destructuring: [$before, $after, $users]
    Client->>PaginatedResource: Named access: ["users" => $users]
    Client->>PaginatedResource: Fluent access: $result->users
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants